Coverage decreased (-0.2%) to 95.557% when pulling 4ce5823 on runtime-polymorphic-shapes into 548782b on master.

Coverage decreased (-0.02%) to 95.752% when pulling 6fe3aea on runtime-polymorphic-shapes into 548782b on master.

Coverage increased (+0.2%) to 95.959% when pulling 5a786c3 on runtime-polymorphic-shapes into 548782b on master.

Coverage increased (+0.3%) to 96.034% when pulling d66b576 on runtime-polymorphic-shapes into 548782b on master.

nice.... mind refreshing the public api docs to reflect changes?

@ninnghazad I've updated the public API docs now. Sorry about not having done that before. Glad to learn they're being looked at! 😄

Incidentally, do you have a preference for things like names of builder methods, like DiskShapeConf{}.UseRadius(3_m)? I've been debating whether to drop their Use prefixes (so the former example would become DiskShapeConf{}.Radius(3_m). That'd make these method names shorter of course but I'd been keeping to a standard of naming methods with verbs and such a change would break that. OTOH I suppose I could drop the verb requirement on methods that are only reading or writing member variables.

Thanks. Ye, i actually use the API docs.
Hm i thought about that a while back - while shortness is nice, i actually do prefer a unified verb approach. So i don't really like the UseThing and would probably prefer SetThing in line with the other Get/Set functions. But then again i'd rather have no verb than "Use" - kind of reminds me of my famous do() functions... cough
I get why it's called like that - but, preferences aside, it does stand out a little from the other names.

EDIT: Oh, and i just remembered about the *Confs - i'd actually like to keep direct access to the (public) members available - it makes writing serializers easier - using Get/Set i have to write a loader and saver, using direct access (or fetchable reference) i can do with a single function for both. Well i could inject friend-lines into the playrho source - but i would want to avoid that.

I picked Use as the name prefix to distinguish builder style setter methods that can be chained. I've used Set as the prefix for setter methods that return void. I'm not convinced that this naming convention helps enough to keep it and I'd mentioned I'm considering dropping the verb all together since methods being non-const and taking a parameter are already indicative of being mutating methods. If people prefer keeping the verb convention but always using the prefix of Set (and not sometime using Use for builder setters), that's important to me and something worth changing to IMO.

Regarding making serialization easier: I'd like that too. Sounds like methods on the shape configuration classes like PolygonShapeConf::Set(vertices) which takes a Span or a VertexSet aren't as convenient as you'd like. Is that correct? I believe all of the shape configuration class do offer all the necessary information however (via getter like methods or public access to data) to be able to manually serialize and deserialize them entirely. They should be fully copy constructible and fully copy assignable too.

What format(s) are you serializing/deserializing to/from? Would full stream I/O support for the << and >> operators be helpful?

More background...

At one point I had in mind that a std::vector could be implicitly converted to a Span so that the PolygonShapeConf::Set method could seemingly just as easily taken a std::vector too. The Span template class has since fallen behind in this regard and I've been considering pulling in the entirety of the GSL code instead (not just the span concept). In any case, I don't mind making it easier for the PolygonShapeConf::Set(vertices) like methods to take more container types.

Dropping all of the methods that enforce invariants on the shape configuration classes is more problematic though. Is there a way serialization can be made easier without opening invariants to being compromised?

A polygon for example can be openly configured from a public vector of points (vertices). Unfortunately the collision detection and handling code can only handle convex polygons made up of vertices in counter-clockwise order however. Additionally that code uses the normals of the edges that adjacent vertices represent which requires slower calculations like doing square roots. So the code calculates edge normals and caches them along with the vertices. Direct access to all of the shape configuration classes' data member means that these requirements can't be enforced and would have to rely on user discipline making these classes easier to use incorrectly.

Use/Verb/Naming: I see, and understand your reasoning. For no other reason but personal taste i don't like chaining in c++ - just think it doesn't read well. How about UseThing (or just Thing) for a chaining-version and SetThing for a void-returning classic setter? Compiler would probably do away with the return of the chaining version if it is not used - but i think it's not a bad idea. More work though.

I thought about the serialization/access/invariant/initialization stuff. To me it seems to depend on how we imagine playrho is used. There might be direkt and immediate creation of shapes/bodies. Which is where i would expect invariants to be needed, and metadata to be generated. Then there might be different cases of indirect and/or deferred creation of objects.

So consider a client server networked game. Client throws a triangle. Through script playrho is called to prepare the triangle and the physics-system spawns it. The same happens on the server upon receiving the client's input event. Now the server simulates the triangle flying for a while and then serializes all relevant state (and probably all information needed to spawn it, in case other clients get to see it). The serialized state gets send to Client which deserializes it and needs to apply the state to its existing triangle, or if needed create the triangle and update it to the supplied state.

In that example we have "normal" creation by calling playrho through script, we have creation from serialized definitions/configs and we have updating from serialized state. For the latter cases i would want as much control as possible over what checking and fancy magic happens behind the scenes - the server will never send bad polygons - the invariants have alread made sure of that upon initial creation. For the first case i would want playrho to make sure i scripted up nice shapes so i can be sure of that from there on. Pretty much the same goes for state written to disk and later loaded, although that's less critical.

So what i'm trying to say in many words is that creation with rules and enforcement is nice, but raw setting for replication is also needed.

stream operators, hm, not sure - in the end i serialize only basic types, so there is control over portability. the serialization in my case is kind of independent from the actual format the data is stored or transmitted in, from a coders perspective. i use https://github.com/USCiLab/cereal and it is awesome.

might look like this:

template<class Archive> void serialize(Archive & archive, FixtureDef & m)
{
        archive( m.isSensor , m.filter );
}

template<class Archive> void serialize(Archive & archive, ShapeDef & m)
{
        archive( m.vertexRadius, m.friction, m.restitution, m.density );
}

template<class Archive> void save(Archive & archive, PolygonShapeConf const & m)
{
        archive(cereal::base_class<ShapeDef>(&m),m.GetVertices());
}

template<class Archive> void load(Archive & archive, PolygonShapeConf & m)
{
        playrho::Span<playrho::Length2> vertices;
        archive(cereal::base_class<ShapeDef>(&m),vertices);
        m.UseVertices(vertices);
}

those serialize() up there can save and load, and are real fast at that. they kind of need to be. the load save would work, but would probably recheck vertices.
maybe friend-ing would be easier, but it feels like cheating and messy to keep updated going forward.

How about UseThing (or just Thing) for a chaining-version and SetThing for a void-returning classic setter? Compiler would probably do away with the return of the chaining version if it is not used - but i think it's not a bad idea. More work though.

Let's take the DiskShapeConf as an example. Are you suggesting that it have both of the following methods for setting the location field?

1. DiskShapeConf& DiskShapeConf::UseLocation(Length2 value) noexcept;
2. void DiskShapeConf::SetLocation(Length2 value) noexcept;

I'd thought about doing both before but decided against it. The only reason that sits well for me for having methods besides protecting invariants, is for supporting chaining. From that perspective, that's in part for supporting constexpr configurations or those that can be entirely expressed within a call to creating a fixture. For example:

body->CreateFixture(DiskShapeConf{}.UseRadius(1_m).UseDensity(1_kgpm2));

I'd not want to stop someone from doing the following though if they're not a fan of chaining and the following is indeed currently possible:

auto conf = DiskShapeConf{};
conf.vertexRadius = 1_m;
conf.density = 1_kgpm2;
body->CreateFixture(conf);

I'm looking at cereal and the serialization. Going to take me some more time absorbing that though. I'll guess that the last example you provided won't work as is since the playrho::Span thingy doesn't allocate any space nor instantiate an object that dynamically does.

Oh wow... I just had an interesting thought regarding the tension between encapsulation and efficiency that's related to this discussion.

My reluctance to providing direct access as I mentioned was due to ensuring the protection of invariants. But that interest doesn't extend into other people's code; only to making PlayRho easier to use correctly (and harder to use incorrectly).

With this new runtime polymorphic design, I believe there's nothing to stop a user from developing their own shapes within their own namespace so long as all the necessary functions are made available suitably for finding according to C++ name resolution rules.

Let's take a custom polygon shape as an example:

namespace foobar {
    struct PolygonConf {
        std::vector<playrho::Length2> vertices;
        std::vector<playrho::UnitVec2> normals;
    };
    
    playrho::ChildCounter GetChildCount(const PolygonConf& arg) noexcept { return 1; }
    
    inline playrho::DistanceProxy
    GetChild(const PolygonConf& arg, playrho::ChildCounter index) {
        return playrho::DistanceProxy{
            0.001f * playrho::Meter,
            static_cast<playrho::VertexCount>(arg.vertices.size()),
            arg.vertices.data(), arg.normals.data()
        };
    }
    
    // also need GetMassData() function implemented
} // namespace foobar

int main() {
    auto world = playrho::World{};
    const auto body = world.CreateBody();
    auto pconf = foobar::PolygonConf{};
    // deserialize data into pconf
    body->CreateFixture(pconf);
}

Could it be that easy?? LOL, I haven't tried with actual code yet so I'm not certain.

Anyways, this also relates to another issue I've been investigating.

hm. need to let that sink in and think about it over the weekend. hadn't crossed my mind to do custom shapes... hm.